UnderlineNav: Improve flickering by moving icon visibility control to a one-way CSS animation#8108
UnderlineNav: Improve flickering by moving icon visibility control to a one-way CSS animation#8108iansan5653 wants to merge 18 commits into
UnderlineNav: Improve flickering by moving icon visibility control to a one-way CSS animation#8108Conversation
🦋 Changeset detectedLatest commit: fc168b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Pull request overview
This PR updates UnderlineNav to avoid SSR→hydration icon flicker by removing the JS “has ever overflowed” state and attempting to move icon-hiding logic entirely into CSS via scroll-driven animations.
Changes:
- Removed
hasEverOverflowedstate and thedata-hide-iconsattribute fromUnderlineNav.tsx. - Added a new
hide-iconsanimation inUnderlineNav.module.css, intended to run once and then keep icons hidden permanently by controlling animation play state via the existing scroll-driven overflow detection. - Minor string literal change for the overflow menu button
aria-label.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/UnderlineNav/UnderlineNav.tsx | Removes JS-based icon hiding (hasEverOverflowed / data-hide-icons) to rely on CSS-only behavior. |
| packages/react/src/UnderlineNav/UnderlineNav.module.css | Adds a one-shot animation approach intended to hide icons without JS and prevent overflow-driven flicker loops. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Low
|
UnderlineNav: Fix flickering by moving icon visibility control to pure CSSUnderlineNav: Fix flickering by moving icon visibility control to a one-way CSS animation
…react into fix-underlinenav-flicker
|
Integration test results from github/github-ui PR:
CI check runs linting, type checking, and unit tests. Check the workflow logs for specific failures. VRT check ensures that when visual differences are detected, the PR cannot proceed until someone acknowledges the changes by adding the "visual difference acknowledged" label. Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance. |
UnderlineNav: Fix flickering by moving icon visibility control to a one-way CSS animationUnderlineNav: Improve flickering by moving icon visibility control to a one-way CSS animation
The icons in
UnderlineNavcan cause flickering as they cause/un-cause the container to overflow when hidden/shown. We use JS to update state to prevent this flickering, but that doesn't run until after hydration. This means that, for certain screen sizes, the icons will flicker after SSR and before client-size hydration.This PR moves the icon visibility logic to a one-way CSS animation on supported browsers. This way, when the scroll-driven animation is triggered, it triggers the corresponding icon-hiding animation, which persists and keeps the icons hidden for the remaining life of the component.
This improves the performance significantly by shifting the calculations into CSS in supported browsers (ie, not in Firefox). This reduces the visibility of the flickering. However, unfortunately it does not completely resolve the flickering; the browser still has to paint the tabs in order to determine the overflow and then hide the icons.
Changelog
New
Changed
UnderlineNavRemoved
Rollout strategy
Testing & Reviewing
Merge checklist